Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add foundations for Android support #587

Merged
merged 3 commits into from
Feb 16, 2023

Conversation

TheCoryBarker
Copy link
Contributor

@TheCoryBarker TheCoryBarker commented Jan 5, 2023

This includes the necessary changes to the native launcher, driver and runtime, but does not yet include a way to instrument the application under test - Android doesn't support Java agents.

Since building the Android targets requires a local installation of the Android SDK and NDK, these targets aren't built by default and only requested in the Linux CI pipeline.

@TheCoryBarker TheCoryBarker marked this pull request as draft January 5, 2023 02:59
@fmeum fmeum self-requested a review January 5, 2023 10:02
.bazelrc Outdated Show resolved Hide resolved
WORKSPACE.bazel Outdated Show resolved Hide resolved
WORKSPACE.bazel Outdated Show resolved Hide resolved
WORKSPACE.bazel Outdated Show resolved Hide resolved
launcher/jazzer_main.cpp Outdated Show resolved Hide resolved
@TheCoryBarker TheCoryBarker marked this pull request as ready for review January 11, 2023 20:20
launcher/AndroidManifest.xml Outdated Show resolved Hide resolved
launcher/jvm_tooling.cpp Outdated Show resolved Hide resolved
launcher/jvm_tooling.cpp Show resolved Hide resolved
launcher/jvm_tooling.cpp Outdated Show resolved Hide resolved
sanitizers/BUILD.bazel Outdated Show resolved Hide resolved
src/main/java/com/code_intelligence/jazzer/Jazzer.java Outdated Show resolved Hide resolved
src/main/java/com/code_intelligence/jazzer/Jazzer.java Outdated Show resolved Hide resolved
src/main/java/com/code_intelligence/jazzer/Jazzer.java Outdated Show resolved Hide resolved
src/main/java/com/code_intelligence/jazzer/Jazzer.java Outdated Show resolved Hide resolved
@fmeum
Copy link
Contributor

fmeum commented Jan 24, 2023

Don't worry about the conflicts, I can fix those when we are done with the review.

@TheCoryBarker
Copy link
Contributor Author

fixed comments

.idea/.gitignore Outdated Show resolved Hide resolved
src/main/java/com/code_intelligence/jazzer/Jazzer.java Outdated Show resolved Hide resolved
launcher/jvm_tooling.cpp Outdated Show resolved Hide resolved
launcher/jvm_tooling.cpp Outdated Show resolved Hide resolved
@fmeum
Copy link
Contributor

fmeum commented Feb 1, 2023

@TheCoryBarker I will resolve the conflicts and then check the CI status.

@TheCoryBarker
Copy link
Contributor Author

There is an issue with non-Android builds and ANDROID_NDK_HOME needing to be set. For me this is not an issue but for most people this will be. I will look into a fix for this

@fmeum
Copy link
Contributor

fmeum commented Feb 2, 2023

@TheCoryBarker I merged in the recent changes from main.

@TheCoryBarker
Copy link
Contributor Author

ANDROID_NDK_HOME and ANDROID_HOME needing to be set is fixed now, however, the test to build all targets will fail for Android targets. How can we fix this?

@fmeum
Copy link
Contributor

fmeum commented Feb 6, 2023

You could move the android targets to a subpackage and exclude that target in CI. Let me know of you want me to take a look at that.

@TheCoryBarker
Copy link
Contributor Author

I found this as another solution which would probably work better in the long run. Can we update the CI command to run:

bazel query '//... except kind(android.*, //...)' | xargs bazel build

@fmeum
Copy link
Contributor

fmeum commented Feb 8, 2023

We can do that if nothing else works, but I would like to preserve the assumption that bazel build //... works locally and also that core Jazzer can be built as an external dependency of other Bazel projects. The latter means that packages providing targets relevant for core Jazzer must not load from unrelated rulesets (e.g. rules_android), which may anyway require moving Android targets to a different package.

@TheCoryBarker If you want, I can take a look and propose a fix.

@TheCoryBarker
Copy link
Contributor Author

I moved the Android build targets into their own package, but I'm not sure how to exclude them from being built in //..., if you could propose a fix that would be really great

@fmeum fmeum force-pushed the AddAndroidSupport branch from 1756fbb to cc0e0db Compare February 12, 2023 09:46
@fmeum
Copy link
Contributor

fmeum commented Feb 12, 2023

I made it so that Android targets aren't built locally unless requested explicitly, but are built as part of the Linux CI build. I also made it so that the android_configure rule disables itself on Windows.

@TheCoryBarker Windows CI runs are failing since jvm_tooling.cpp unconditionally includes dlfcn.h. Maybe replace the bool android parameter with preprocessor conditionals?

@TheCoryBarker
Copy link
Contributor Author

Thank you, I am having a very tough time getting a Windows machine with correct permissions to test my changes on...

@TheCoryBarker
Copy link
Contributor Author

Thanks for the suggestion Fabian, I updated to use preprocessor conditionals

@fmeum
Copy link
Contributor

fmeum commented Feb 14, 2023

@TheCoryBarker I fixed some minor issues, CI looks good now. I left a single comment and will merge when it's resolved.

@fmeum fmeum force-pushed the AddAndroidSupport branch 2 times, most recently from d2fbd98 to 2656ade Compare February 15, 2023 08:21
@fmeum fmeum changed the title Add android support Add foundations for Android support Feb 15, 2023
This includes the necessary changes to the native launcher, driver and
runtime, but does not yet include a way to instrument the application
under test - Android doesn't support Java agents.

Since building the Android targets requires a local installation of the
Android SDK and NDK, these targets aren't built by default and only
requested in the Linux CI pipeline.
@fmeum fmeum force-pushed the AddAndroidSupport branch from 2656ade to 906c224 Compare February 15, 2023 08:22
@fmeum fmeum enabled auto-merge February 15, 2023 08:22
@fmeum
Copy link
Contributor

fmeum commented Feb 15, 2023

@TheCoryBarker Thanks for resolving that problem. I squashed the commits, added a commit message and hit merge.

@fmeum fmeum disabled auto-merge February 15, 2023 08:23
@fmeum fmeum enabled auto-merge (squash) February 16, 2023 11:45
@fmeum fmeum disabled auto-merge February 16, 2023 12:36
@fmeum fmeum enabled auto-merge (squash) February 16, 2023 12:36
@fmeum fmeum disabled auto-merge February 16, 2023 12:47
@fmeum fmeum enabled auto-merge (squash) February 16, 2023 12:47
@fmeum fmeum merged commit 3d149d3 into CodeIntelligenceTesting:main Feb 16, 2023
kmnls pushed a commit to kmnls/jazzer that referenced this pull request Apr 13, 2023
This includes the necessary changes to the native launcher, driver and
runtime, but does not yet include a way to instrument the application
under test - Android doesn't support Java agents.

Since building the Android targets requires a local installation of the
Android SDK and NDK, these targets aren't built by default and only
requested in the Linux CI pipeline.

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
@TheCoryBarker TheCoryBarker deleted the AddAndroidSupport branch April 1, 2024 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants